Skip to content

Fix stack recursion on udp resolve - #292

Closed
zeyus wants to merge 1 commit into
sccn:devfrom
NexusDynamic:feature/fix-udp-recursion
Closed

Fix stack recursion on udp resolve#292
zeyus wants to merge 1 commit into
sccn:devfrom
NexusDynamic:feature/fix-udp-recursion

Conversation

@zeyus

@zeyus zeyus commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Often I came across an issue when doing some testing that freezes the entire network stack when using liblsl with a lot of inlets / outlets and a larger portrange (e.g. 128) on my local machine, both in Linux and MacOS.

I think the problem is a combination of number of ports, number of interfaces (I have too many, uni VPNs, VMs, etc), and clients - especially when running locally testing multiple nodes.

This patch keeps the void resolve_attempt_udp::send_next_query method a shallow loop rather than becoming recursive, and seems to fix the problem.

I ran a set of tests using my dart bindings with the patch and unpatched version and you can see that the patched version doesn't change the number of calls which unfortunately goes up with the number of ports (as expected), but there's a huge difference in the recursion depth.

                    max_depth                                         max_stack_bytes
                        count  mean    std  min  25%  50%  75%    max           count     mean       std  min  25%  50%    75%       max
portRange status
16        patched       516.0  1.00   0.00  1.0  1.0  1.0  1.0    1.0           516.0     0.00      0.00  0.0  0.0  0.0    0.0       0.0
          unpatched     516.0  3.05  15.28  1.0  1.0  1.0  2.0  175.0           516.0  1051.78   7822.88  0.0  0.0  0.0  512.0   89088.0
32        patched       768.0  1.00   0.00  1.0  1.0  1.0  1.0    1.0           768.0     0.00      0.00  0.0  0.0  0.0    0.0       0.0
          unpatched     769.0  3.21  24.04  1.0  1.0  1.0  1.0  335.0           769.0  1130.53  12307.47  0.0  0.0  0.0    0.0  171008.0
64        patched      1280.0  1.00   0.00  1.0  1.0  1.0  1.0    1.0          1280.0     0.00      0.00  0.0  0.0  0.0    0.0       0.0
          unpatched    1281.0  3.32  36.50  1.0  1.0  1.0  1.0  655.0          1281.0  1190.27  18687.76  0.0  0.0  0.0    0.0  334848.0

The test was done with a continuous resolver, using the following script (and I added some logguru logging in the resolve_attempt_udp.cpp source):

import 'dart:io';
import 'package:liblsl/lsl.dart';

Future<void> main() async {
  final portRange = int.parse(Platform.environment['POC_PORT_RANGE'] ?? '16');

  LSL.setConfigContent(
    LSLApiConfig(
      sessionId: 'stackpoc',
      portRange: portRange,
      ipv6: IPv6Mode.allow,
      knownPeers: const ['127.0.0.1'],
      logLevel: 0,
    ),
  );

  final info = await LSL.createStreamInfo(
    streamName: 'PocStream',
    streamType: LSLContentType.markers,
    channelCount: 1,
    sampleRate: 0,
    channelFormat: LSLChannelFormat.int32,
    sourceId: 'poc_src',
  );
  final outlet = await LSL.createOutlet(streamInfo: info);

  final resolver = LSL.createContinuousStreamResolver(
    forgetAfter: 30.0,
    maxStreams: 20,
  );
  for (var i = 0; i < 3; i++) {
    await resolver.resolve(waitTime: 2.0);
  }
  resolver.destroy();
  outlet.destroy();
  stdout.writeln('POC_OK portRange=$portRange');
}

Full results table: stack.csv

@zeyus

zeyus commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

I think this is complementary to #283 , this doesn't fix the crash, but this avoids stack recursion

@cboulay

cboulay commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

I rebased this change onto current dev (ab5e080b) and resolved the conflict in send_next_query(), retaining:

  • the iterative traversal from this PR;
  • query sending through the receive socket (matching the advertised reply port);
  • nonthrowing outbound-interface selection and its logging;
  • the cancellation guard before querying the socket endpoint, plus the recently merged cancellation fixes.

The rebased commit retains the original author: e73aaea9. It is available on sccn/liblsl:review/pr292-rebased, with the diff against dev.

I could not update this PR's head branch: maintainer edits are disabled and my account has no push access to the fork. The branch above is ready to use for updating the source branch or integrating the rebased change.

Validation on macOS: Release build with LSL_UNITTESTS=ON, LSL_FRAMEWORK=OFF; all four dedicated resolver regression executables passed (bad interface, advertised query port, same-machine discovery, TCP fallback), as did all three exported [resolver] tests: 12 assertions across 7 cases. git diff --check passed. No Windows/Linux runtime tests were run locally.

@cboulay cboulay mentioned this pull request Sep 19, 2026
Merged
cboulay added a commit that referenced this pull request Sep 19, 2026
@cboulay
Merge pull request #296 from sccn/review/pr292-rebased
59ede11 
Avoid recursive UDP resolve traversal (rebased #292)
@cboulay

cboulay commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

superseded by #296

@cboulay cboulay closed this Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zeyus @cboulay